-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add policy to align engines fields with ci #289
base: master
Are you sure you want to change the base?
Conversation
UlisesGascon
commented
Oct 22, 2024
•
edited
Loading
edited
- closes: Discussion: Using engines in the package.json #286
- related: Re-thinking the discussion process #285
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
we need a note in here about the possibility of continuing to run CI in older unsupported node versions so we know if/when a change clearly breaks back-compat. (we do this for express itself, for example) I'll try to come up with good wording for this |
ping @ctcpip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While I have general concerns about the decision here, I would much prefer this decision over the current state. The only thing I think is missing is the call-out that all more restrictive engines changes can only land with a MAJOR. And that we agreed major versions should not be cut just to change node support.
This means that this policy will take a while to roll out. I do not consider this blocking to merge this ADR, but I think it is something we had agreed on in a few discussions so just wanted to call it out since I didn't see it mentioned in the text of the ADR.